-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[charts] Add watermark on the pro ResponsiveChartContainer
#13398
Conversation
Deploy preview: https://deploy-preview-13398--material-ui-x.netlify.app/ |
Sorry but I don't understand the difference 😆 |
😅 Effectively, I copy pasted the same
|
We should probably follow what the other packages do |
But I noticed each package does a different thing 😅 |
I would probably go with the DataGrid solution if we want to always load the new functionalities on the |
If people need to use the pro version (which is the case here), I'd keep the |
Yeah, it's hard to be 100% consistent on this topic, the trade-off vary greatly across the packages. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, should we also block the use of the ResponsiveChartContainer
?
The |
I'm hesitating between reexporting the container like I'm currently doing (
<ResponsiveChartContainer />
), or exporting them with a pro suffix<ResponsiveChartContainerPro />
.@flaviendelangle Do you have some insight on this aspect? the reexport only add watermark, and it's still unsure if we will add behavior to this component